Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colorscheme refactoring and fixes #179

Merged
merged 17 commits into from
May 25, 2023
Merged

Conversation

gdjohn4s
Copy link
Member

Description

As stated in the title, this PR aims to improve the code regarding the color scheme.

Proposed Changes

In detail:

  • Color detection
  • Use of classes instead of data-theme
  • Removed "dark" state, now only "default" and "light" exist
  • Code cleanup
  • Removed unused code

The reason for these changes is code optimization.

Checklist

  • My submission is formatted according to the guidelines in the contributing guide
  • My addition is ordered alphabetically
  • My submission has a useful description
  • Code follows the established CODE_OF_CONDUCT guidelines.
  • Code has been tested thoroughly and passes all tests.
  • All commit messages are descriptive and follow the established commit message format.
  • Pull request title follows the established pull request title format.
  • Pull request description clearly describes the changes included in the pull request.
  • The description does not have more than 100 characters
  • Each table column is padded with one space on either side
  • I have searched the repository for any relevant issues or pull requests
  • Any category I am creating has the minimum requirement of 3 items

@gdjohn4s gdjohn4s added documentation Improvements or additions to documentation enhancement New feature or request frontend labels May 24, 2023
@gdjohn4s gdjohn4s requested a review from FlavioAdamo May 24, 2023 21:58
setThemeElements(theme);
theme.value = setTheme();
const isDarkTheme: boolean =
theme.value === "dark" || theme.value === null ? true : false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- theme.value === "dark" || theme.value === null ? true : false;
+ theme.value === "dark" || theme.value === null 

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 21 to 26
export const getThemeElements = (theme: globalThis.Ref<string>): boolean => {
if (theme.value === null || theme.value === 'dark') {
return true
}
document.querySelector("body")?.setAttribute("data-theme", theme.value);
return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be replaced with

+ return theme.value === null || theme.value === 'dark'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@FlavioAdamo FlavioAdamo merged commit 1b03c6f into main May 25, 2023
@FlavioAdamo FlavioAdamo deleted the colorscheme_refactoring_and_fixes branch May 25, 2023 11:17
@gdjohn4s gdjohn4s mentioned this pull request Jun 1, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants